Skip to content

Add rate limiting support through policy_effect extension#1673

Open
Copilot wants to merge 7 commits intomasterfrom
copilot/add-rate-limiting-policy-effect
Open

Add rate limiting support through policy_effect extension#1673
Copilot wants to merge 7 commits intomasterfrom
copilot/add-rate-limiting-policy-effect

Conversation

Copy link
Contributor

Copilot AI commented Jan 8, 2026

Rate Limiting Feature - Implementation Complete ✅

This PR successfully implements native rate limiting support in Casbin through policy_effect extension.

✅ Implementation Checklist

  • Understand codebase structure (effector interface, policy effect parsing, enforcement flow)
  • Create RateLimitEffector with rate limiting logic
    • Parse rate_limit() expression from policy_effect
    • Implement bucket management (counters and time windows)
    • Implement MergeEffects with rate limit checks
    • Thread-safe implementation with proper mutex locking
  • Pass request context (sub, obj, act) to effector for bucket key generation
  • Create comprehensive tests
    • 11 unit tests covering all functionality
    • 4 integration tests with enforcer
    • Different time units (second, minute, hour, day)
    • Different count types (allow, all)
    • Different bucket types (sub, obj, act, all)
    • Race condition testing
  • Run full test suite - all tests passing ✅
  • Create example application with working demo ✅
  • Create comprehensive documentation ✅
  • Address all code review feedback ✅
    • Fix race condition in bucket key generation
    • Fix import formatting
    • Improve comment clarity for critical functions
    • Add proper error handling in tests
    • Fix golangci-lint godot errors (comments ending with periods)
    • Fix golangci-lint style/complexity errors:
      • Remove unused nolint directive
      • Fix type inference (stylecheck)
      • Fix shadow variable declaration
      • Reduce nested complexity with helper function
      • Fix goimports formatting
  • Security scan - no vulnerabilities ✅
  • All golangci-lint checks passing ✅

📊 Test Results

All tests passing:

  • ✅ 11 unit tests in effector package
  • ✅ 4 integration tests
  • ✅ All existing tests (no regressions)
  • ✅ Race detector clean
  • ✅ CodeQL security scan clean
  • ✅ golangci-lint clean

🎯 Key Features Delivered

  1. Flexible Configuration: Support for multiple time units, count types, and bucket types
  2. Thread-Safe: Proper mutex locking prevents race conditions
  3. Context-Aware: Automatic bucket key generation from request parameters
  4. Backward Compatible: Existing code continues to work without changes
  5. Well Tested: Comprehensive test coverage
  6. Well Documented: Example application and detailed README
  7. Clean Code: Passes all linter checks

📝 Usage

// Enable rate limiting
rateLimitEft := effector.NewRateLimitEffector()
e.SetEffector(rateLimitEft)

// Model configuration
// [policy_effect]
// e = rate_limit(3, second, allow, sub)

🔒 Security

No security vulnerabilities introduced. All inputs validated, proper error handling, thread-safe implementation.

Ready for merge! 🚀

Original prompt

This section details on the original issue you should resolve

<issue_title>[feature] Support rate limiting through policy_effect extension</issue_title>
<issue_description>It would be great if Casbin could support rate limiting natively by extending the policy_effect expression and implementing a custom Effector.

The idea is to introduce a new rate_limit function in policy_effect like this:

[policy_effect]
e = rate_limit(10, minute, allow, sub)

This means "allow at most 10 requests per minute, only count allowed requests, and bucket by subject". The four parameters are:

  • max: maximum request count within the time window
  • unit: time window unit, could be second, minute, hour, or day
  • count_type: either allow (only count allowed requests) or all (count all requests including denied ones, useful for preventing brute-force attacks)
  • bucket: the field(s) to group by, such as all, sub, obj, act

A RateLimitEffector implementing the Effector interface would parse this expression, maintain internal bucket states (counters and window timestamps), and decide whether to allow or deny based on current count. The bucket key is generated from the request context according to the bucket type, for example sub:alice when bucket is sub.

To implement this, we need to extend the policy_effect expression parser to recognize the rate_limit function, create a new RateLimitEffector that implements the Effector interface with internal state management, and make sure the enforce context (sub, obj, act) is passed to the effector so it can generate proper bucket keys.</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@hsluoyz hsluoyz marked this pull request as ready for review January 8, 2026 17:55
Copilot AI and others added 4 commits January 8, 2026 18:02
Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>
Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>
Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>
Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>
Copilot AI changed the title [WIP] Add support for rate limiting in policy_effect extension Add rate limiting support through policy_effect extension Jan 8, 2026
Copilot AI requested a review from hsluoyz January 8, 2026 18:13
@hsluoyz
Copy link
Member

hsluoyz commented Jan 9, 2026

@copilot fix:

Annotations
10 errors and 2 warnings
[golangci: effector/rate_limit_effector.go#L223](https://github.com/casbin/casbin/pull/1673/files#annotation_43903354831)
Comment should end in a period (godot)
[golangci: effector/rate_limit_effector.go#L211](https://github.com/casbin/casbin/pull/1673/files#annotation_43903354836)
Comment should end in a period (godot)
[golangci: effector/rate_limit_effector.go#L186](https://github.com/casbin/casbin/pull/1673/files#annotation_43903354842)
Comment should end in a period (godot)
[golangci: effector/rate_limit_effector.go#L176](https://github.com/casbin/casbin/pull/1673/files#annotation_43903354847)
Comment should end in a period (godot)
[golangci: effector/rate_limit_effector.go#L103](https://github.com/casbin/casbin/pull/1673/files#annotation_43903354853)
Comment should end in a period (godot)
[golangci: effector/rate_limit_effector.go#L87](https://github.com/casbin/casbin/pull/1673/files#annotation_43903354858)
Comment should end in a period (godot)
[golangci: effector/rate_limit_effector.go#L51](https://github.com/casbin/casbin/pull/1673/files#annotation_43903354862)
Comment should end in a period (godot)
[golangci: effector/rate_limit_effector.go#L40](https://github.com/casbin/casbin/pull/1673/files#annotation_43903354867)
Comment should end in a period (godot)
[golangci: effector/rate_limit_effector.go#L33](https://github.com/casbin/casbin/pull/1673/files#annotation_43903354874)
Comment should end in a period (godot)
[golangci: effector/rate_limit_effector.go#L27](https://github.com/casbin/casbin/pull/1673/files#annotation_43903354878)
Comment should end in a period (godot)

Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jan 9, 2026

@copilot fix:

...

Fixed in 67c4d14. Added periods to all comment endings to satisfy the godot linter rule.

@hsluoyz
Copy link
Member

hsluoyz commented Jan 9, 2026

@copilot fix:

<html>
<body>
<!--StartFragment-->
golangciissues found
--
golangci: effector/effector.go#L15directive `//nolint:cyclop // TODO` is unused for linter "cyclop" (nolintlint)
golangci: effector/rate_limit_effector.go#L116ST1023: should omit type Effect from declaration; it will be inferred from the right-hand side (stylecheck)
golangci: rate_limit_test.go#L38shadow: declaration of "err" shadows declaration at line 26 (govet)
golangci: enforcer.go#L801`if ok` has complex nested blocks (complexity: 9) (nestif)
golangci: rate_limit_test.go#L102File is not `goimports`-ed (goimports)
golangci: enforcer.go#L803File is not `goimports`-ed (goimports)

<!--EndFragment-->
</body>
</html>[golangci](https://github.com/casbin/casbin/actions/runs/20850860739/job/59905625758#step:4:32)
issues found
[golangci: effector/effector.go#L15](https://github.com/casbin/casbin/pull/1673/files#annotation_43904171907)
directive `//nolint:cyclop // TODO` is unused for linter "cyclop" (nolintlint)
[golangci: effector/rate_limit_effector.go#L116](https://github.com/casbin/casbin/pull/1673/files#annotation_43904171910)
ST1023: should omit type Effect from declaration; it will be inferred from the right-hand side (stylecheck)
[golangci: rate_limit_test.go#L38](https://github.com/casbin/casbin/pull/1673/files#annotation_43904171912)
shadow: declaration of "err" shadows declaration at line 26 (govet)
[golangci: enforcer.go#L801](https://github.com/casbin/casbin/pull/1673/files#annotation_43904171918)
`if ok` has complex nested blocks (complexity: 9) (nestif)
[golangci: rate_limit_test.go#L102](https://github.com/casbin/casbin/pull/1673/files#annotation_43904171920)
File is not `goimports`-ed (goimports)
[golangci: enforcer.go#L803](https://github.com/casbin/casbin/pull/1673/files#annotation_43904171924)
File is not `goimports`-ed (goimports)

…hadow var, reduce nesting

Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jan 9, 2026

@copilot fix:

...

Fixed in 922f56e:

  • Removed unused nolint:cyclop directive from effector.go
  • Fixed type inference (removed explicit types from var declarations)
  • Fixed shadow variable declaration (renamed err to err2 in loop)
  • Reduced nested complexity by extracting helper function extractRequestParam()
  • Fixed goimports formatting (removed trailing whitespace)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feature] Support rate limiting through policy_effect extension

3 participants